feat: Discourage assert(Not)Empty if "empty" usage is disallowed - #325
feat: Discourage assert(Not)Empty if "empty" usage is disallowed#325mitelg wants to merge 3 commits into
Conversation
410764b to
927cf58
Compare
927cf58 to
812be4d
Compare
3ac6163 to
589b9ed
Compare
| PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule: | ||
| phpstan.rules.rule: [%strictRulesInstalled%, %featureToggles.bleedingEdge%] | ||
| PHPStan\Rules\PHPUnit\AssertEmptyIsDiscouragedRule: | ||
| phpstan.rules.rule: [%strictRulesInstalled%, %strictRules.disallowedEmpty%] |
There was a problem hiding this comment.
Should it be also behind %featureToggles.bleedingEdge% ?
There was a problem hiding this comment.
good catch, I think you are right.
| @@ -12,6 +12,8 @@ rules: | |||
| conditionalTags: | |||
| PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule: | |||
| phpstan.rules.rule: [%strictRulesInstalled%, %featureToggles.bleedingEdge%] | |||
There was a problem hiding this comment.
I am not sure what you are referring to? AssertEqualIsDiscouraged is configured with [%strictRulesInstalled%, %featureToggles.bleedingEdge%]?
There was a problem hiding this comment.
I mean it should be with
[%strictRulesInstalled%, %strictRules.disallowedLooseComparison%, %featureToggles.bleedingEdge%]
There was a problem hiding this comment.
I think we cannot use %strictRules.disallowedLooseComparison% because it would result into a fatal error when strict rules are not installed.
There was a problem hiding this comment.
%strictRulesInstalled% is a flag which is available because it is known in phpstan-src.
we cannot depend on other 1st party config flags which are only available when a extension is installed
There was a problem hiding this comment.
[%strictRulesInstalled%, %strictRules.disallowedEmpty%] is not valid
| } elseif ($node instanceof FuncCall) { | ||
| if (!$node->name instanceof Name || !in_array(strtolower($scope->resolveName($node->name)), ['phpunit\\framework\\assertempty', 'phpunit\\framework\\assertnotempty'], true)) { | ||
| return []; | ||
| } | ||
| } else { | ||
| return []; | ||
| } |
There was a problem hiding this comment.
function based assertions seem to be not supported by other Assert* rules, so I think we shouldn't start here.
it would need a separate PR doing it in all rules (not sure its worth adding,... noone asked for these yet AFAIR)
3 participants
fixes: #270
Hey,
first attempt to fix the linked issue. I used Codex for that and I hope it did the job correct 😁 Tests are looking fine for me.
Happy to get some feedback 🤝
Best regards 👋